Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Fix for https://github.com/FineUploader/fine-uploader/issues/1172 #1964

Merged
merged 4 commits into from
Jan 27, 2018
Merged

Conversation

galvinhsiu
Copy link
Contributor

@galvinhsiu galvinhsiu commented Jan 21, 2018

Brief description of the changes

Fix for the long standing issue on uploading using chunking,
this usually kicked in when uploading very large files to S3 (1+
gigabytes). This patch alleviates the need to inflate the autoretry attempt number to be huge number, which may have ramifications on the end user.

The change is on a successful upload chunk, if autoretry is turned on and the upload is not cancelled or paused to reset the autoretry counter to 0 for the given upload handle.

Please refer to:
(#1172).

Recommendations / comments are welcome.

What browsers and operating systems have you tested these changes on?

{example: Safari on iOS 9.1.0 and IE11 on Windows 8.1}

Safari on Mac Sierra
< Firefox 50, > Firefox 50 on Mac Sierra
IE11 on Windows 10 / 8
Edge 14, Edge 15+ on Windows 10

Have you written unit tests? If not, explain why.

Yes, see attached code. Additional test added to fail out each chunk exactly once and then retry successfully, with the check on the attempt number = 1.

fixes #1172

@galvinhsiu galvinhsiu changed the title Patch / Fix for https://github.com/FineUploader/fine-uploader/issues/1172 Fix for https://github.com/FineUploader/fine-uploader/issues/1172 Jan 21, 2018
Copy link
Member

@rnicholus rnicholus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, but if you could address my two comments...

@@ -197,6 +200,86 @@ if (qqtest.canDownloadFileAsBlob) {
});
}

function testChunkedEveryFailureAndRecovery(done) {
assert.expect(6 + (expectedChunks * 17) + (expectedChunks * 6), done);

This comment was marked as spam.

_onUploadChunkSuccess: function(id, chunkData) {
var uploadData = this._uploadData.retrieve({id: id});

if (!this._preventRetries[id] && this._options.retry.enableAuto && uploadData.status !== qq.status.PAUSED) {

This comment was marked as spam.

@galvinhsiu
Copy link
Contributor Author

Good feedback, ripped out the unused condition and slightly tweaked the unit test.

rnicholus
rnicholus previously approved these changes Jan 24, 2018
@rnicholus
Copy link
Member

Awesome, thanks! I'd like to get this merged in, and maybe you can do that. Let's continue our discussion on Twitter DM.

@rnicholus rnicholus merged commit 401482e into FineUploader:master Jan 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

retry count does not reset with chunked uploads to S3
3 participants